Skip to content

Add InfluxDB timeseries storage for system metrics#24

Open
jbingham17 wants to merge 2 commits intomainfrom
add-influxdb-timeseries
Open

Add InfluxDB timeseries storage for system metrics#24
jbingham17 wants to merge 2 commits intomainfrom
add-influxdb-timeseries

Conversation

@jbingham17
Copy link
Contributor

@jbingham17 jbingham17 commented Mar 9, 2026

Summary

  • Add InfluxDB integration to persist CPU, memory, load average, and process count as timeseries data points on each metrics collection
  • Add new /api/history endpoint for querying historical metrics with configurable measurement, time range, and field parameters
  • Gracefully disabled when INFLUX_TOKEN env var is not set — existing functionality is unaffected

Test plan

  • Verify server starts normally without InfluxDB configured (no INFLUX_TOKEN)
  • Set INFLUX_TOKEN, INFLUX_URL, INFLUX_ORG, INFLUX_BUCKET and verify data is written to InfluxDB
  • Query /api/history?measurement=cpu&range=-1h and verify timeseries data is returned
  • Confirm /api/history returns 503 when InfluxDB is not configured

🤖 Generated with Claude Code

Summary by CodeRabbit

  • New Features

    • InfluxDB integration for persistent metrics storage and historical data querying
    • New /api/history endpoint enables querying historical metrics with customizable measurement, time range, and field parameters; requires InfluxDB configuration
    • Server debug mode for enhanced diagnostics and logging
  • Chores

    • Added InfluxDB client dependency

jbingham17 and others added 2 commits March 9, 2026 11:56
Adds a --debug CLI flag that enables timestamped debug logging for
request handling, CPU/memory collection, and process fetching.
Includes server:debug and start:debug npm scripts for convenience.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Write CPU, memory, load average, and process count data points to
InfluxDB on each metrics collection. Add /api/history endpoint for
querying historical data. Gracefully disabled when INFLUX_TOKEN is
not set.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@coderabbitai
Copy link

coderabbitai bot commented Mar 9, 2026

📝 Walkthrough

Walkthrough

This change adds InfluxDB integration to the metrics server, enabling metrics persistence and historical querying. It introduces a new /api/history endpoint, debug mode with command-line flag support, and environment-driven configuration for InfluxDB connectivity.

Changes

Cohort / File(s) Summary
Package Configuration
package.json
Added InfluxDB client dependency (@influxdata/influxdb-client v1.35.0) and two new debug scripts (server:debug and start:debug).
InfluxDB Module
server/influxdb.ts
New module providing InfluxDB integration: initialization via environment variables, metric writing with aggregated CPU/memory/load/process data, Flux query-based metric retrieval, and lifecycle management.
Server Integration
server/index.ts
Enhanced with InfluxDB initialization at startup, debug flag parsing, debug logging throughout metric collection, metric writing on /api/metrics requests, and new /api/history endpoint for querying historical data with measurement/range/field parameters.

Sequence Diagram

sequenceDiagram
    participant Client
    participant Server
    participant InfluxDB
    
    rect rgba(0, 100, 200, 0.5)
    Note over Server: Startup
    Server->>InfluxDB: initInfluxDB() - create client
    InfluxDB-->>Server: connected (if INFLUX_TOKEN set)
    end
    
    rect rgba(0, 150, 100, 0.5)
    Note over Server: Metric Collection & Write
    Client->>Server: GET /api/metrics
    Server->>Server: collect CPU, memory, load metrics
    Server->>InfluxDB: writeMetrics(aggregated data)
    InfluxDB-->>Server: ack
    Server-->>Client: metrics JSON
    end
    
    rect rgba(150, 100, 0, 0.5)
    Note over Server: Historical Query
    Client->>Server: GET /api/history?measurement=cpu&range=-1h
    Server->>InfluxDB: queryMetrics(measurement, range, field)
    InfluxDB-->>Server: time-value pairs
    Server-->>Client: historical data JSON
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes


Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Min-Level ❌ Error The queryMetrics function in server/influxdb.ts directly interpolates user-supplied parameters into Flux query strings without validation, enabling query injection attacks. Implement input validation with allow-lists for measurements and fields, validate range format with regex, and sanitize all parameters before query construction.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the main change: adding InfluxDB integration for persisting system metrics.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch add-influxdb-timeseries

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (4)
server/influxdb.ts (3)

147-151: Consider resetting module state in closeInfluxDB.

When closing the InfluxDB connection, queryApi and enabled are not reset. If the module is reinitialized after close, stale state could cause issues. Additionally, closeInfluxDB should handle errors gracefully.

♻️ Proposed improvement
 export async function closeInfluxDB(): Promise<void> {
   if (writeApi) {
-    await writeApi.close();
+    try {
+      await writeApi.close();
+    } finally {
+      writeApi = null;
+      queryApi = null;
+      enabled = false;
+    }
   }
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/influxdb.ts` around lines 147 - 151, In closeInfluxDB, after awaiting
writeApi.close(), also null out or reset module-level state variables queryApi
and writeApi and set enabled to false so the module can be safely reinitialized;
wrap the close logic in a try/catch to log or handle errors (without throwing)
so cleanup doesn't crash the process and ensure the catch clears state as well
(referencing closeInfluxDB, writeApi, queryApi, and enabled).

35-43: Consider extracting the metrics type to src/types.ts.

The inline type definition for the metrics parameter duplicates the structure already partially defined in src/types.ts (which has CpuUsage and SystemMetrics). Extracting a shared type would improve maintainability.

Based on the relevant code snippet from src/types.ts, the CpuUsage interface already matches the cpuUsage array element type here.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/influxdb.ts` around lines 35 - 43, The writeMetrics function uses an
inline metrics type that duplicates existing types; instead import CpuUsage and
SystemMetrics (or a combined SystemMetrics-like interface) from src/types.ts and
replace the inline parameter type in writeMetrics with the shared type (e.g.,
metrics: SystemMetrics or a new alias that combines CpuUsage array and the
memory/load/process fields). Update the function signature in writeMetrics and
add the appropriate import at the top so the function refers to the centralized
types instead of the inline definition.

98-101: Non-blocking flush swallows errors silently.

The .catch() logs the error message but doesn't propagate the failure. While this prevents blocking the request, consider adding a counter or metric for write failures to aid debugging in production.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/influxdb.ts` around lines 98 - 101, The current non-blocking flush
(writeApi.flush().catch(...)) only logs err.message and swallows failures;
update the catch handler for writeApi.flush() to (1) log the full error (err)
with context, and (2) increment or record a write-failure metric/counter (e.g.,
call metrics.influxWriteFailures.inc() or influxWriteFailures.increment()) so
failures are observable in production; ensure the metric is imported/initialized
(or created alongside existing metrics) and do not rethrow to keep flush
non-blocking.
server/index.ts (1)

22-23: Consider logging the initialization result.

initInfluxDB() returns a boolean indicating success, but the return value is ignored. While the function logs its own status, capturing and using the result could be useful for conditional startup behavior or diagnostics.

♻️ Suggested improvement
 // Initialize InfluxDB timeseries storage
-initInfluxDB(debug);
+const influxInitialized = initInfluxDB(debug);
+if (DEBUG) {
+  debug("InfluxDB initialization:", influxInitialized ? "success" : "skipped/failed");
+}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/index.ts` around lines 22 - 23, The call to initInfluxDB(debug)
ignores its boolean return; capture its result (e.g., const influxInitialized =
initInfluxDB(debug)) and log or act on it so startup can respond to failure—use
the returned value to emit a clear processLogger/info or error message and
optionally adjust startup flow (e.g., exit or continue degraded) based on
influxInitialized; reference the initInfluxDB symbol where the call is made and
update nearby startup logic to handle the boolean result.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@server/influxdb.ts`:
- Around line 113-120: The Flux query is vulnerable because measurement, range,
and field are directly interpolated into the query string; validate and sanitize
these inputs before building the query (e.g., enforce a strict whitelist/regex
for measurement and field like /^[A-Za-z0-9_.-]+$/ or map them against known
allowed names, and validate range is a safe numeric/ISO value or convert it
server-side rather than accepting raw strings). Update the code that constructs
the query (the template using INFLUX_BUCKET, measurement, field, range) to
reject or sanitize any values failing the checks and only use validated values
when interpolating to eliminate possible Flux injection.

---

Nitpick comments:
In `@server/index.ts`:
- Around line 22-23: The call to initInfluxDB(debug) ignores its boolean return;
capture its result (e.g., const influxInitialized = initInfluxDB(debug)) and log
or act on it so startup can respond to failure—use the returned value to emit a
clear processLogger/info or error message and optionally adjust startup flow
(e.g., exit or continue degraded) based on influxInitialized; reference the
initInfluxDB symbol where the call is made and update nearby startup logic to
handle the boolean result.

In `@server/influxdb.ts`:
- Around line 147-151: In closeInfluxDB, after awaiting writeApi.close(), also
null out or reset module-level state variables queryApi and writeApi and set
enabled to false so the module can be safely reinitialized; wrap the close logic
in a try/catch to log or handle errors (without throwing) so cleanup doesn't
crash the process and ensure the catch clears state as well (referencing
closeInfluxDB, writeApi, queryApi, and enabled).
- Around line 35-43: The writeMetrics function uses an inline metrics type that
duplicates existing types; instead import CpuUsage and SystemMetrics (or a
combined SystemMetrics-like interface) from src/types.ts and replace the inline
parameter type in writeMetrics with the shared type (e.g., metrics:
SystemMetrics or a new alias that combines CpuUsage array and the
memory/load/process fields). Update the function signature in writeMetrics and
add the appropriate import at the top so the function refers to the centralized
types instead of the inline definition.
- Around line 98-101: The current non-blocking flush
(writeApi.flush().catch(...)) only logs err.message and swallows failures;
update the catch handler for writeApi.flush() to (1) log the full error (err)
with context, and (2) increment or record a write-failure metric/counter (e.g.,
call metrics.influxWriteFailures.inc() or influxWriteFailures.increment()) so
failures are observable in production; ensure the metric is imported/initialized
(or created alongside existing metrics) and do not rethrow to keep flush
non-blocking.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 9b1e1e2f-f926-436b-a396-eb805cee1593

📥 Commits

Reviewing files that changed from the base of the PR and between 0e7262e and c80d9c6.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (3)
  • package.json
  • server/index.ts
  • server/influxdb.ts
📜 Review details
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-02-18T05:07:00.671Z
Learnt from: jbingham17
Repo: coderabbit-demo/btop PR: 8
File: src/types.ts:32-32
Timestamp: 2026-02-18T05:07:00.671Z
Learning: In the coderabbit-demo/btop repository, avoid inline comments on TypeScript interface properties. Ensure property types are self-documenting through clear names and interface structure. If a property requires explanation, document it above the interface or with JSDoc comments for the property, not inline on the same line.

Applied to files:

  • server/influxdb.ts
  • server/index.ts
🪛 Biome (2.4.4)
server/influxdb.ts

[error] 23-23: This hook is being called conditionally, but all hooks must be called in the exact same order in every component render.

(lint/correctness/useHookAtTopLevel)

🔇 Additional comments (5)
package.json (2)

9-11: LGTM!

The debug scripts are correctly configured and mirror the non-debug counterparts with the --debug flag appended.


17-17: No action required—version 1.35.0 is valid and has no known direct vulnerabilities.

Version 1.35.0 of @influxdata/influxdb-client is a legitimate stable release with no reported CVEs or published security advisories. The caret constraint (^1.35.0) will permit patch and minor version updates if newer stable releases become available.

server/index.ts (3)

8-20: LGTM!

The debug mode implementation is clean and well-structured. The timestamp formatting provides useful context for debugging, and the flag parsing is straightforward.


309-333: The /api/history endpoint structure looks good.

The endpoint correctly:

  • Returns 503 when InfluxDB is not configured
  • Provides sensible defaults for query parameters
  • Handles errors with appropriate status codes and logging

Note: The input validation concern for measurement, range, and field parameters is addressed in the server/influxdb.ts review comment regarding Flux injection.


297-299: LGTM!

The writeMetrics call is appropriately placed after collecting metrics and before returning the response. Since writeMetrics handles the disabled state internally and uses non-blocking flush, this won't impact response latency.

Comment on lines +113 to +120
const query = `
from(bucket: "${INFLUX_BUCKET}")
|> range(start: ${range})
|> filter(fn: (r) => r._measurement == "${measurement}")
|> filter(fn: (r) => r._field == "${field}")
|> aggregateWindow(every: 10s, fn: mean, createEmpty: false)
|> yield(name: "mean")
`;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical: Flux query injection vulnerability.

The measurement, range, and field parameters are directly interpolated into the Flux query string without sanitization. A malicious user could inject arbitrary Flux code via the /api/history endpoint query parameters.

For example, a request like /api/history?measurement=cpu") |> drop() |> from(bucket: "other could manipulate the query logic.

🔒 Proposed fix: Validate and sanitize inputs
+// Allowed measurements and fields for safe querying
+const ALLOWED_MEASUREMENTS = ["cpu", "cpu_core", "memory", "load", "processes"];
+const ALLOWED_FIELDS = ["usage_percent", "user_percent", "system_percent", "idle_percent", 
+                        "total_bytes", "used_bytes", "free_bytes", "avg_1m", "avg_5m", "avg_15m", "count"];
+const RANGE_PATTERN = /^-\d+[smhdw]$/; // e.g., -1h, -30m, -7d
+
 export async function queryMetrics(
   measurement: string,
   range: string = "-1h",
   field: string = "usage_percent"
 ): Promise<{ time: string; value: number }[]> {
   if (!enabled || !queryApi) {
     return [];
   }

+  // Validate inputs to prevent Flux injection
+  if (!ALLOWED_MEASUREMENTS.includes(measurement)) {
+    throw new Error(`Invalid measurement: ${measurement}`);
+  }
+  if (!ALLOWED_FIELDS.includes(field)) {
+    throw new Error(`Invalid field: ${field}`);
+  }
+  if (!RANGE_PATTERN.test(range)) {
+    throw new Error(`Invalid range format: ${range}`);
+  }
+
   const query = `
     from(bucket: "${INFLUX_BUCKET}")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@server/influxdb.ts` around lines 113 - 120, The Flux query is vulnerable
because measurement, range, and field are directly interpolated into the query
string; validate and sanitize these inputs before building the query (e.g.,
enforce a strict whitelist/regex for measurement and field like
/^[A-Za-z0-9_.-]+$/ or map them against known allowed names, and validate range
is a safe numeric/ISO value or convert it server-side rather than accepting raw
strings). Update the code that constructs the query (the template using
INFLUX_BUCKET, measurement, field, range) to reject or sanitize any values
failing the checks and only use validated values when interpolating to eliminate
possible Flux injection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant